RR Edge ID Verification #3164
RR Edge ID Verification #3164
Conversation
github-actions
bot
added
VPR
There was a problem hiding this comment.
Thanks @amin1377.
I left a few comment for improving the readability a bit.
|
I’d leave it grouped with verification options. I’d also document it in the .rst for consistency.VaughnOn Jul 1, 2025, at 11:35 AM, Amin Mohaghegh ***@***.***> wrote:
@amin1377 commented on this pull request.
In vpr/src/base/read_options.cpp:
@@ -1637,6 +1637,16 @@ argparse::ArgumentParser create_arg_parser(const std::string& prog_name, t_optio
.default_value("on")
.show_in(argparse::ShowIn::HELP_ONLY);
+ gen_grp.add_argument<bool, ParseOnOff>(args.verify_rr_switch_id, "--verify_rr_switch_id")
Strictly speaking, it’s not a routing option. If set to false, it disables a default check and uses the switch ID from the RR graph instead. Because of this, I grouped it with other checks (like verify_file_digests, etc.). However, if you still think it would be better to include it under the router group, please let me know.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because your review was requested.Message ID: ***@***.***>
|
…n update_rr_switch_id
…ecursive function
…to validate_and_update_traceback
|
Thanks @soheilshahrouz and @vaughnbetz for reviewing the code. Please let me know if you have any further suggestions. |
There was a problem hiding this comment.
Thanks. I suggest a couple more changes.
One more thought: should we validate the switch_ids sometimes and not others (what the code is currently doing) or should we just always recompute them on reading in a route file? We don't have to change it, but thought I'd ask ... maybe we're writing out redundant information and then correcting it. In that case not writing out the redundant information at all might be better (although changing the .route format would cause some backwards-incompatibility).
|
|
||
| if (trace->iswitch == OPEN) { //End of a branch | ||
| //Verify that the next element (branch point) has been already seen in the traceback so far | ||
| if (!seen_rr_nodes.count(next->index)) { |
There was a problem hiding this comment.
Should check that index is a valid rr_node index (otherwise the validator could crash on invalid input). Print a helpful error message if it is out of range.
There was a problem hiding this comment.
validate_and_update_traceback is called within the process_nodes function. In this function, all relevant checks related to the node are performed when creating the trace.
|
Ok thanks.
Get Outlook for iOS<https://aka.ms/o0ukef>
________________________________
From: Amin Mohaghegh ***@***.***>
Sent: Tuesday, July 1, 2025 2:34:41 PM
To: verilog-to-routing/vtr-verilog-to-routing ***@***.***>
Cc: Vaughn Betz ***@***.***>; Comment ***@***.***>
Subject: Re: [verilog-to-routing/vtr-verilog-to-routing] RR Edge ID Verification (PR #3164)
@amin1377 commented on this pull request.
________________________________
In vpr/src/base/old_traceback.cpp<#3164 (comment)>:
+ if (trace->iswitch == OPEN) { //End of a branch
//Verify that the next element (branch point) has been already seen in the traceback so far
if (!seen_rr_nodes.count(next->index)) {
validate_and_update_traceback is called within the process_nodes function. In this function, all relevant checks related to the node are performed when creating the trace.
—
Reply to this email directly, view it on GitHub<#3164 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BLE5FN7LQRS3RTL7SII7V4T3GLIEDAVCNFSM6AAAAACABUKD62VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDSNZWGU4TAMBZGY>.
You are receiving this because you commented.Message ID: ***@***.***>
|
I think having this additional check is useful, especially now that we have an option to disable it if needed. In the .route file, the switch ID is also printed, which has actually helped me a few times when debugging routing. Otherwise, I would have had to find the edge ID between two nodes from the RR graph file first to extract the information I needed. |
Description
In a
.routefile, each edge between two nodes includes a switch ID, which is used to retrieve the electrical characteristics of that edge. When reading the.routefile, this switch ID is compared against the corresponding switch ID from the RR Graph for consistency. If they do not match, an error is raised.This consistency check is generally helpful. However, when analyzing multiple timing corners, each corner may have a different RR Graph with varying timing information (graph topologies are the same). As a result, the number and ordering of switch types may differ between corners. To allow reuse of the same
.routefile for fair comparisons across different timing corners, this strict verification can become a limitation.To address this issue, a new CLI parameter
verify_rr_switch_idhas been introduced. By default, it is set toon, enforcing the consistency check. When set tooff, the switch ID from the.routefile is ignored, and the switch ID from the RR Graph is used instead.